sql/opt: allow non-default NULLS ordering with SELECT DISTINCT#171980
Open
shafi-VM wants to merge 1 commit into
Open
sql/opt: allow non-default NULLS ordering with SELECT DISTINCT#171980shafi-VM wants to merge 1 commit into
shafi-VM wants to merge 1 commit into
Conversation
When null_ordered_last is enabled (or an explicit NULLS FIRST/LAST is given), buildOrderBy synthesizes a (col IS NULL) ordering column ahead of each column with non-default NULLS ordering. For a plain SELECT DISTINCT, constructDistinct rejected any ordering column not in the select list, so these synthesized columns triggered a spurious for SELECT DISTINCT, ORDER BY expressions must appear in select list error. The DISTINCT ON path (buildDistinctOn) already tolerates these columns (case 3): a (col IS NULL) expression is functionally determined by col, so when col is a grouping column the synthesized column can be treated as one too without changing the result. This extracts that check into a shared isSynthesizedNullsOrderingCol helper and uses it from both constructDistinct and buildDistinctOn. It fixes user queries such as SET null_ordered_last = on; SELECT DISTINCT a FROM t ORDER BY a; as well as DROP USER and DROP ROLE, which internally run a SELECT DISTINCT ... ORDER BY against system.privileges. Fixing constructDistinct rather than suppressing the session setting for internal queries is the correct layer, since the error reproduces with the plain user query above. Fixes cockroachdb#168317 Epic: none Release note (bug fix): Fixed a bug that caused SELECT DISTINCT ... ORDER BY queries to fail with a "for SELECT DISTINCT, ORDER BY expressions must appear in select list" error when the null_ordered_last session setting was enabled. The same error caused DROP USER and DROP ROLE statements to fail, since they run such a query internally.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
null_ordered_lastis enabled (or explicitNULLS FIRST/LASTis used),buildOrderBysynthesizes a(col IS NULL)ordering column ahead of columns with non-default NULL ordering. For plainSELECT DISTINCT,constructDistinctrejected these synthesized columns because they were not in the select list, causing a spurious:error.
buildDistinctOnalready handles this case by treating(col IS NULL)as functionally determined bycol. This extracts that logic into a sharedisSynthesizedNullsOrderingColhelper and reuses it in bothconstructDistinctandbuildDistinctOn.This fixes queries such as:
It also fixes
DROP USERandDROP ROLE, which internally execute aSELECT DISTINCT ... ORDER BYagainstsystem.privileges.Fixes #168317
Epic: none
Release note (bug fix): Fixed a bug that caused
SELECT DISTINCT ... ORDER BYqueries to fail with a "for SELECT DISTINCT, ORDER BY expressions must appear in select list" error whennull_ordered_lastwas enabled. The same issue also causedDROP USERandDROP ROLEto fail because they execute such queries internally.